Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/8.0-rc1] Fix NullableAttribute illink test failures #90680

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 16, 2023

Backport of #90449 to release/8.0-rc1

/cc @carlossanlop @sbomer

Customer Impact

No known impact. This fixes test bugs that were exposed by an update to a new SDK. The fix is relevant for internal testing purposes but does not reflect a customer scenario.

Testing

This fixes test issues that were fixed in main, but not in rc1.

Risk

Very low risk. No change to product behavior.

When we started building with preview 7 in
5549f72, NullableAttribute in
these testcases started to use the attribute definition from the
framework, instead of generating it into the code. This broke the
`--used-attrs-only` optimization because `skip` assemblies (the
default for the framework in these testcases) are treated as if
all types in them are kept, for the purposes of the
`--used-attrs-only` optimization. (The optimization removes
attribute instances unless the attribute type is preserved for
some other reason).

It's not clear what the intended behavior of `--used-attrs-only`
is for `skip` assemblies, and the discussion in
dotnet/linker#952 indicates that it's
considered experimental, so this fixes the failures by using the
`link` action. This represents a more realistic scenario since
`skip` is mainly used in testing to avoid linking the framework
in every testcase.
@github-actions github-actions bot requested a review from marek-safar as a code owner August 16, 2023 18:17
@ghost ghost added the linkable-framework Issues associated with delivering a linker friendly framework label Aug 16, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 16, 2023
@ghost
Copy link

ghost commented Aug 16, 2023

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #90449 to release/8.0-rc1

/cc @carlossanlop @sbomer

Customer Impact

Testing

Risk

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

linkable-framework, needs-area-label

Milestone: -

@carlossanlop
Copy link
Member

This is currently blocking the arcade dependency flow PR for RC1: #90665

@carlossanlop carlossanlop added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 16, 2023
@ghost
Copy link

ghost commented Aug 16, 2023

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #90449 to release/8.0-rc1

/cc @carlossanlop @sbomer

Customer Impact

Testing

Risk

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

linkable-framework, area-Tools-ILLink

Milestone: -

@carlossanlop carlossanlop added this to the 8.0.0 milestone Aug 16, 2023
@steveisok steveisok self-requested a review August 16, 2023 18:20
@carlossanlop carlossanlop added Servicing-consider Issue for next servicing release review Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 16, 2023
@carlossanlop
Copy link
Member

Test-only change.

@carlossanlop carlossanlop merged commit dbb6f45 into release/8.0-rc1 Aug 16, 2023
@carlossanlop carlossanlop deleted the backport/pr-90449-to-release/8.0-rc1 branch August 16, 2023 18:44
@ghost ghost locked as resolved and limited conversation to collaborators Sep 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers linkable-framework Issues associated with delivering a linker friendly framework Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants